Skip to content

Conversation

@wking
Copy link
Contributor

@wking wking commented May 11, 2018

containernetworking/plugins@a0eac8d7 (pkg/ns: remove namespace creation, 2018-03-16) removed NewNS, which we use in libpod/networking.go. Pinning to the previous commit, containernetworking/plugins@1fb94a42 (Merge pull request #96 from DennisDenuto/denuto/master, 2018-03-14), allows us to run vndr without breaking our build. This is a short term fix; moving forward we'll want to either drop this dependency or catch up with the new upstream API.

The upstream package seems to have been fairly stable in the meantime, because even with the new pinned version, a vndr re-vendor generates no changes:

$ vndr github.com/containernetworking/plugins

Spun off from #747.

@wking
Copy link
Contributor Author

wking commented May 11, 2018

I'd guess the test failures are flakes? I don't see how changing the vendor.conf without anything in vendor changing would lead to:

fatal error: runtime: netpoll failed

@giuseppe
Copy link
Member

bot, retest this please

@rh-atomic-bot
Copy link
Collaborator

☔ The latest upstream changes (presumably a160857) made this pull request unmergeable. Please resolve the merge conflicts.

@mheon
Copy link
Member

mheon commented May 11, 2018

Tests are green, but you picked up a merge conflict

containernetworking/plugins@a0eac8d7 (pkg/ns: remove namespace
creation, 2018-03-16) removed NewNS, which we use in
libpod/networking.go.  Pinning to the previous commit,
containernetworking/plugins@1fb94a42 (Merge pull request containers#96 from
DennisDenuto/denuto/master, 2018-03-14), allows us to run vndr without
breaking our build.  This is a short term fix; moving forward we'll
want to either drop this dependency or catch up with the new upstream
API.

The upstream package seems to have been fairly stable in the meantime,
because even with the new pinned version, a vndr re-vendor generates
no changes:

  $ vndr github.com/containernetworking/plugins

Signed-off-by: W. Trevor King <[email protected]>
@wking wking force-pushed the cni-plugins-pin branch from cd1d970 to 49d597f Compare May 11, 2018 14:56
@wking
Copy link
Contributor Author

wking commented May 11, 2018

Rebased.

@mheon
Copy link
Member

mheon commented May 11, 2018

LGTM
@rh-atomic-bot r+

@rh-atomic-bot
Copy link
Collaborator

📌 Commit 49d597f has been approved by mheon

@rh-atomic-bot
Copy link
Collaborator

⌛ Testing commit 49d597f with merge 608f0b9...

@rh-atomic-bot
Copy link
Collaborator

☀️ Test successful - status-papr
Approved by: mheon
Pushing 608f0b9 to master...

@wking wking deleted the cni-plugins-pin branch May 11, 2018 16:08
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 27, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants